chore: deprecate IE detection helpers in FeatureDetection #13133
Conversation
Deprecate FeatureDetection.isInternetExplorer and FeatureDetection.internetExplorerVersion now that Cesium no longer supports Internet Explorer. Refs #12615
|
Thank you for the pull request, @MohammadShujaullah! Welcome to the Cesium community! In order for us to review your PR, please complete the following steps:
Review Pull Request Guidelines to make sure your PR gets accepted quickly. |
|
@cla-bot check |
jjspace
left a comment
jjspace
left a comment
There was a problem hiding this comment.
Thanks for the PR @MohammadShujaullah, I can confirm we have the CLA on file for you. Feel free to add yourself to CONTRIBUTORS.md as well.
First off it seems this PR has the changes from your other PRs included. Can you please trim it down to only the relevant changes for this PR to keep things clean and make it easier to review and merge?
MohammadShujaullah
force-pushed
the
deprecate-ie-feature-detection
branch
from
362247f to
f18382e
Compare
|
@MohammadShujaullah is this ready for another review? |
|
yes it is ready for review sir |
jjspace
left a comment
jjspace
left a comment
There was a problem hiding this comment.
Overall the new removals look good. Thanks @MohammadShujaullah.
It looks like we're still bumping into the geometry JSDoc changes on this branch as well. See comment from your other PR: #13134 (comment)
Looking into it a bit more it looks like you have a commit on the fork's main branch which is what's getting included in all your PRs. This may not be as easy to see from the git log since it's not actually on any of your specific branches.
---
config:
gitGraph:
mainBranchName: 'cesium-main'
---
gitGraph
commit id: "9e1f702"
branch fork-main
checkout fork-main
commit id: "b63382d - clarify Geometry..."
branch fix-sandcastle-links
checkout fix-sandcastle-links
commit id: "4194d6e - deprecated IE det..."
commit id: "f18382 - completly remove IE..."
| isPrimary: options.isPrimary ?? 0, | ||
| }); | ||
| } | ||
| event = new window.PointerEvent(type, { |
There was a problem hiding this comment.
Now that this isn't actually modified it can probably become a const assignment. Maybe even a direct return
|
I haven't dissected the Git history issues and changes here. From quickly scrolling over the files, it looks like the relevant changes here are affecting "few" files (e.g. the changes related to #13131 are rather isolated). Two thoughts for resolving this:
Yeah, both approaches are very "pragmatic", but ... inspired by https://xkcd.com/1597/ , when there are the options to sweat and tremble while doing some bisect/rebase/cherryPick black Git magic, or just do some copy+paste to have one clean commit with close-to-zero effort, I'm usually leaning towards the latter ... |
3 participants
Fixes #12615
Description
Deprecates the Internet Explorer detection helpers in
FeatureDetection:FeatureDetection.isInternetExplorer()now always returnsfalseand is marked@deprecated.FeatureDetection.internetExplorerVersion()now always returnsundefinedand is marked@deprecated.This also removes now-unnecessary user agent parsing for Internet Explorer, simplifying the feature detection code path.
Cesium no longer supports Internet Explorer, so these helpers no longer need to perform real detection.
Testing